Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3.x] Add Forge facade for Laravel integration #177

Merged
merged 9 commits into from
Jun 27, 2024
Merged

[3.x] Add Forge facade for Laravel integration #177

merged 9 commits into from
Jun 27, 2024

Conversation

stevebauman
Copy link
Contributor

@stevebauman stevebauman commented Jun 2, 2024 •
edited
Loading

This PR adds a Forge facade for Laravel integration, allowing users to call Forge API methods using a singleton with an API token defined in their services.php configuration file:

// config/services.php

'forge' => [
    'token' => env('FORGE_API_TOKEN'),
],
// Use Forge...

use Laravel\Forge\Facades\Forge;

$daemon = Forge::createDaemon('server-id', [...]);

I did not add illuminate/support to the composer.json, as I'm sure we don't want to make it a requirement for users of the SDK, but anyone installing this into a Laravel application will be able to utilize the facade as soon as they add an API key to their env/config.

This also has the added benefit of easy testing due to the Facade integration:

// tests/DaemonTest.php

use Laravel\Forge\Facades\Forge;

Forge::shouldReceive('createDaemon')->once('...');

If this is something you'd still like to avoid having in the SDK, no worries at all! Really appreciate your time ❤️

@driesvints driesvints requested a review from jbrooksuk June 3, 2024 07:27
stevebauman
stevebauman commented Jun 4, 2024
View reviewed changes
src/Facades/Forge.php Outdated Show resolved Hide resolved
@jbrooksuk
Copy link
Member

This is great, thanks @stevebauman. Let's get the Facade documentor added, then we can look to merge this.

Setting to draft for now.

@jbrooksuk jbrooksuk marked this pull request as draft June 5, 2024 13:56
@stevebauman
Copy link
Contributor Author

Ok sounds good @jbrooksuk! I've added the new GitHub action, though it may need approval to be ran (it doesn't look like it's being executed in this PR), so I've ran it manually on my end for now. Static analysis will also fail right now, not sure if how we should handle that.

Let me know if you have any other feedback or things you'd like adjusted, thanks for your time!

@stevebauman stevebauman marked this pull request as ready for review June 6, 2024 15:38
@driesvints
Copy link
Member

@stevebauman the workflow will only be executed on a push to master or an *.x branch. This is ok and intended.

Btw, you may want to add the illuminate/support package to the dev dependencies to fix the static analysis issue.

@stevebauman
Copy link
Contributor Author

Ah awesome thanks @driesvints! Looks like we're all green now! 🎉

@driesvints driesvints requested a review from taylorotwell June 25, 2024 21:14
@jbrooksuk jbrooksuk merged commit 777b254 into laravel:3.x Jun 27, 2024
8 checks passed
@navjotsinghprince
Copy link

A Great Thanks for add Forge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siarheipashkevich siarheipashkevich siarheipashkevich left review comments

@jbrooksuk jbrooksuk jbrooksuk approved these changes

@taylorotwell taylorotwell Awaiting requested review from taylorotwell

Assignees

@jbrooksuk jbrooksuk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stevebauman @jbrooksuk @driesvints @navjotsinghprince @siarheipashkevich